Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Conditions from apimachinery, specifically k8s.io/apimachinery/pk… #1644

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

joshuatcasey
Copy link
Member

@joshuatcasey joshuatcasey commented Aug 27, 2023

Use Conditions from apimachinery, specifically k8s.io/apimachinery/pkg/apis/meta/v1.Conditions

Probably try to merge #1646 and make all required pipeline changes before running this through the pipelines.

Release note:

This includes a small breaking change for consumers of the generated client code.

If consumers directly import one of the removed `Conditions` structs, they will need to import the `Condition` struct from the `k8s.io/apimachinery/pkg/apis/meta/v1` library.

@codecov
Copy link

codecov bot commented Aug 27, 2023

Codecov Report

Merging #1644 (64f1bff) into main (96fcfe4) will increase coverage by 0.01%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main    #1644      +/-   ##
==========================================
+ Coverage   75.53%   75.55%   +0.01%     
==========================================
  Files         166      166              
  Lines       15159    15159              
==========================================
+ Hits        11451    11454       +3     
+ Misses       3406     3404       -2     
+ Partials      302      301       -1     
Files Changed Coverage Δ
...sorconfig/oidcclientwatcher/oidc_client_watcher.go 87.65% <ø> (ø)
internal/testutil/oidcclient.go 0.00% <ø> (ø)
...onfig/ldapupstreamwatcher/ldap_upstream_watcher.go 75.86% <66.66%> (ø)
...streamwatcher/active_directory_upstream_watcher.go 90.07% <88.88%> (ø)
...onfig/oidcupstreamwatcher/oidc_upstream_watcher.go 95.06% <93.75%> (ø)

... and 2 files with indirect coverage changes

@joshuatcasey joshuatcasey force-pushed the jtc/use-conditions-from-apimachinery branch 2 times, most recently from b290a8a to 3da8f00 Compare August 28, 2023 02:08
@joshuatcasey joshuatcasey changed the base branch from main to multiple_idps_and_transformations August 28, 2023 19:28
@joshuatcasey joshuatcasey changed the base branch from multiple_idps_and_transformations to main August 28, 2023 19:30
@joshuatcasey joshuatcasey force-pushed the jtc/use-conditions-from-apimachinery branch 3 times, most recently from cc91203 to 039a163 Compare August 29, 2023 21:39
@joshuatcasey joshuatcasey force-pushed the jtc/use-conditions-from-apimachinery branch from 039a163 to b1bd05d Compare September 7, 2023 14:49
Copy link
Member

@cfryanr cfryanr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but can we hold this back until we merge #1419 due to the expected merge conflicts?

@cfryanr cfryanr force-pushed the jtc/use-conditions-from-apimachinery branch from b1bd05d to 34cc78d Compare September 11, 2023 17:10
@cfryanr
Copy link
Member

cfryanr commented Sep 11, 2023

I looked at what the merge conflicts would be in detail by running a trial merge and they aren't that bad, so I think we can go ahead and merge this now. I rebased main into the PR branch and will mark it for auto-merge.

@cfryanr cfryanr enabled auto-merge September 11, 2023 17:12
@cfryanr cfryanr force-pushed the jtc/use-conditions-from-apimachinery branch from 34cc78d to 64f1bff Compare September 11, 2023 17:16
@cfryanr cfryanr merged commit fee737b into main Sep 11, 2023
@cfryanr cfryanr deleted the jtc/use-conditions-from-apimachinery branch September 11, 2023 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants